Defer the inclusion until after ActiveRecord has been fully loaded, improving booting performance and following Rails best practices#118
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR defers the loading of Rails Settings components until ActiveRecord is fully loaded, improving application boot performance by avoiding premature ActiveRecord initialization. This follows Rails best practices for gem loading order.
Key changes:
- Wraps ActiveRecord extensions in
ActiveSupport.on_load(:active_record)hook - Moves
rails-settings/setting_objectrequire inside the deferred loading block
lib/rails-settings.rb
Outdated
| def self.has_settings(*args, &block) | ||
| RailsSettings::Configuration.new(*args.unshift(self), &block) | ||
| ActiveSupport.on_load(:active_record) do | ||
| require 'rails-settings/setting_object' |
There was a problem hiding this comment.
The require statement for 'rails-settings/setting_object' is moved inside the on_load block, but there's no explanation for why this specific require needs to be deferred while others (lines 11-12) remain outside. Consider documenting why this require depends on ActiveRecord being loaded or evaluating if other requires should also be moved.
| require 'rails-settings/setting_object' |
There was a problem hiding this comment.
I think this line should be deleted, because it's already in line 1
ledermann
left a comment
ledermann
left a comment
There was a problem hiding this comment.
Thanks for contributing!
Before merging, the duplicated require should be removed.
lib/rails-settings.rb
Outdated
| def self.has_settings(*args, &block) | ||
| RailsSettings::Configuration.new(*args.unshift(self), &block) | ||
| ActiveSupport.on_load(:active_record) do | ||
| require 'rails-settings/setting_object' |
There was a problem hiding this comment.
I think this line should be deleted, because it's already in line 1
…mproving booting performance and following Rails best practices
shashankdass
force-pushed
the
master_lazy-load-activerecordbase-includes
branch
from
8c36728 to
0d3da71
Compare
|
Updated the PR. Removing the duplicate. |
2 participants
Issue Description
The current implementation references ActiveRecord on gem load which triggers premature loading of Active Record. This can significantly slow down application boot time, especially in development and test environments.
Solution
Use Rails recommended ActiveSupport.on_load(:active_record) hook to defer module inclusion until after ActiveRecord is fully loaded. This improves boot performance and avoids potential load order issues in edge cases.